Skip to content

Fix warnings #2881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 31, 2017
Merged

Fix warnings #2881

merged 7 commits into from
Jan 31, 2017

Conversation

rodionkvashnin
Copy link
Contributor

Hello!
I fixed some warnings related to unused variables and uninitialized fields. I think not everyone is Core developer, so users should not see warnings in Core code when they compile their sketches. Related issue: #2587.

@codecov-io
Copy link

codecov-io commented Jan 19, 2017

Codecov Report

Merging #2881 into master will increase coverage by 0.01%.

@@            Coverage Diff             @@
##           master    #2881      +/-   ##
==========================================
+ Coverage    27.8%   27.82%   +0.01%     
==========================================
  Files          20       20              
  Lines        3625     3626       +1     
  Branches      656      656              
==========================================
+ Hits         1008     1009       +1     
  Misses       2441     2441              
  Partials      176      176
Impacted Files Coverage Δ
cores/esp8266/spiffs/spiffs_hydrogen.c 38.34% <ø> (ø)
cores/esp8266/spiffs_api.h 44.7% <66.66%> (+0.32%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d85e783...d55efd3. Read the comment docs.

Copy link
Member

@igrr igrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning up the warnings!
I wonder if we can tweak build flags when doing Travis build to error out on warnings in the first place...

Just a few notes about struct initializers — i think memset will be a better option, as the current way doesn't look very maintainable :)

@@ -49,7 +49,7 @@ class SPIFFSImpl : public FSImpl
{
public:
SPIFFSImpl(uint32_t start, uint32_t size, uint32_t pageSize, uint32_t blockSize, uint32_t maxOpenFds)
: _fs({0})
: _fs{{0, 0, 0, 0, 0, 0, 0, 0}, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we remove this initializer and use memset instead?

@@ -176,7 +176,7 @@ class SPIFFSImpl : public FSImpl

bool _tryMount()
{
spiffs_config config = {0};
spiffs_config config = {0, 0, 0, 0, 0, 0, 0, 0};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here — memset will look slightly cleaner

@@ -268,7 +273,7 @@ class SPIFFSFileImpl : public FileImpl
SPIFFSFileImpl(SPIFFSImpl* fs, spiffs_file fd)
: _fs(fs)
, _fd(fd)
, _stat({0})
, _stat{0, 0, 0, 0, {0}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likewise

@@ -376,7 +381,7 @@ class SPIFFSFileImpl : public FileImpl
auto rc = SPIFFS_fstat(_fs->getFs(), _fd, &_stat);
if (rc != SPIFFS_OK) {
DEBUGV("SPIFFS_fstat rc=%d\r\n", rc);
_stat = {0};
_stat = {0, 0, 0, 0, {0}};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likewise

@igrr igrr merged commit 2126146 into esp8266:master Jan 31, 2017
@rodionkvashnin rodionkvashnin deleted the fix-warnings branch January 31, 2017 16:28
Lauszus added a commit to felis/USB_Host_Shield_2.0 that referenced this pull request Feb 1, 2017
This should be removed when the ESP8266 core is updated in PlatformIO: esp8266/Arduino#2881
Lauszus added a commit to felis/USB_Host_Shield_2.0 that referenced this pull request Feb 1, 2017
This should be removed when the ESP8266 core is updated in PlatformIO: esp8266/Arduino#2881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants